Skip to content

fix(spec): the defaultValue literal gate prescribes the key rename, not a missing-member type error - #16409

Merged
huangyiirene merged 5 commits into
mainfrom
claude/issue-16077-default-value-literal-rename-preference
Sep 7, 2026
Merged

fix(spec): the defaultValue literal gate prescribes the key rename, not a missing-member type error#16409
huangyiirene merged 5 commits into
mainfrom
claude/issue-16077-default-value-literal-rename-preference

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Fixes #16077

checkLiteralDefaultValue — the shared core of the FieldSchema.defaultValue gate (#7127) and the ActionParamSchema.defaultValue gate (#6970) — read a value-contract rejection positionally, result.error.issues[0]. zod reports per-member issues before the object-level unrecognized_keys one, so when an author RENAMED a key the message naming the rename sorted last and was discarded, and the author was handed a missing-member type error about a member they never wrote.

The gate now prefers the unrecognized_keys issue when the rejection carries one — the same shape #15490 landed on the stored-value path, implemented separately here because packages/spec is upstream of objectql and cannot import that helper.

Reproduced on current main before touching anything

The card's three reproductions were measured at c463d03e0. Re-measured on origin/main at de75e407e5 rather than inherited — all three still hold, byte-identical to the card:

call detail before after
{type:'location'}, {latitude:1, longitude:2} Invalid input: expected number, received undefined names `latitude` -> `lat`, `longitude` -> `lng`
{type:'address'}, {street:5, postal_code:'98101'} Invalid input: expected string, received number names `postal_code` -> `postalCode`
{type:'address'}, {postal_code:'98101'} already named the rename unchanged

The third row is the point: which diagnosis an author got depended on whether some unrelated member happened to also be wrong. That asymmetry is what this removes.

Prerequisite 1 — the 16-class sweep, re-run ON THIS FUNCTION

#15490's sweep was taken on the record-validator.ts path; that it carries over here was an inference. Measured here instead, over every class valueSchemaFor(def, 'stored') covers, at BOTH arities ({type} and {type, multiple: true}), probing each with object literals carrying undeclared keys:

# class probe type can emit unrecognized_keys?
1 STRING_VALUE_TYPES text no
2 NUMERIC_VALUE_TYPES number no
3 BOOLEAN_VALUE_TYPES boolean no
4 CALENDAR_DATE_TYPES date no
5 INSTANT_TYPES datetime no
6 CLOCK_TIME_TYPES time no
7 option types select no
8 REFERENCE_VALUE_TYPES lookup no
9 FILE_REFERENCE_TYPES file no
10 location location YES
11 address address YES
12 composite composite no
13 record record no
14 repeater repeater no
15 vector vector no
16 open fallback (z.unknown()) json no

2 of 16location and address, the only two backed by a strictObject. The preference is a no-op for the other fourteen, measured on this function rather than carried over. A pin holding that no-op ships with the change.

Prerequisite 2 — callers and pins on the current detail text

Callers of checkLiteralDefaultValue, from grep -rn "checkLiteralDefaultValue" over the whole tree excluding node_modules and dist:

  • packages/spec/src/ui/action.zod.ts:500 — composes verdict.detail into the action-param refusal.
  • packages/spec/src/ui/action.zod.ts:510 — arity retry; reads .ok only, never .detail.
  • packages/spec/src/data/field.zod.ts:2070 — composes verdict.detail into the field refusal.
  • packages/spec/src/data/default-value-shape.test.ts — the unit test.
  • Zero callers outside this repo's packages/spec. Checked objectui at 51a402f: 0 hits.

Assertions pinning the current text: none, and the searches that returned that zero were lit first.

  • default-value-shape.test.ts:98 pins detail for a datetime (toContain('ISO-8601 instant')) — class 5, where unrecognized_keys is unreachable, so unmoved.
  • field-default-value.test.ts already carried an address and a location rejection row, but both are the LUCKY half: their other members are well-typed, so nothing sorted ahead and the rename surfaced by accident. Both still pass.
  • Positive control for the test-tree search: grep -rn -F "ISO-8601 instant" over *.ts/*.tsx returns 16 hits including default-value-shape.test.ts:98 — the search reaches the test tree and can return a hit.
  • Positive control for the message-text search: grep -rn -F "Unrecognized key(s) on this address value" over *.ts,*.tsx,*.md,*.mdx,*.json,*.snap returns 1 hit — the search reaches those file types.
  • Positive control for the objectui search: grep -rln "LocationField" returns 17 files there.

One trap worth recording: the first pass searched for the composed phrase cannot satisfy this field's own stored value contract and got zero — because the producer splits it across template-literal concatenation lines, so it exists on no single line. Zero from a search whose control had not been lit is not a reading; the shorter single-line-safe phrases above are.

Clause 2 — I read it as NO, same as the dispatch

The mechanical floor in references/contract-review.md is a new exported symbol, or a new key on a published payload. This diff has neither:

  • actionableValueIssueMessage is module-local, not exported.
  • LiteralDefaultValueVerdict keeps exactly { ok, detail? } — no new key, no type change.
  • Mechanically confirmed against the generated export ledgers: check:api-surface exit 0 and check:export-origins exit 0 ("5277 exports across 17 entry points resolve exactly as recorded"), with git status clean after the build, so neither ledger moved.

What moves is which of several ALREADY-reachable messages detail carries. The declared meaning — "the 'why' a refusal carries verbatim" — does not move; the implementation is brought into line with it. That is the triage seat's own 声明 ≠ 交付 framing. No verdict moves either: exactly the same defaults are accepted and refused.

The one doc edit is the incidental half of the same TSDoc line, "First issue message", which described the mechanism that was wrong. The load-bearing half is kept verbatim.

Tests

Six new pins, each asserting BOTH halves — the prescription is present AND the half that was shown instead is gone. Without the second half a pin cannot see a regression back to the positional read.

  • default-value-shape.test.ts — the core: missing-member case (location), wrong-typed-member case (address), the asymmetry-is-gone pin holding the lucky and unlucky cases equal, and the no-op pin for a class that cannot emit unrecognized_keys.
  • field-default-value.test.ts — two rows on the existing case table, which grew a notContains column.
  • action-param-default-value.test.ts — one pin, because that consumer composes detail into its own message: a core fix that never reached the composed text would be invisible to a core-only pin.

Ablation. From the committed implementation, the call site was mutated back to the positional read, proven on disk (anchor counts 1 to 0 for the new spelling and 0 to 1 for the old; blob hash differs from the HEAD blob), the three files re-run, then restored via git checkout HEAD -- path and the restore proven by blob-hash equality plus an empty git diff HEAD. Result: exactly the 6 new pins fail, the other 93 in those files pass. Direction: turned red, as predicted. No build was needed — the pins import the subject by relative path within its own package, so vitest reads src/, not dist/.

Verification

Every verdict below is the gate's own printed line with its exit code captured before any pipe.

  • pnpm --filter @objectstack/spec test — exit 0. Test Files 481 passed | 1 skipped (482), Tests 13107 passed | 1 skipped (13108).
  • pnpm --filter @objectstack/spec typecheck — exit 0, all three legs (tsc --noEmit, check:scripts-typecheck, check:test-typecheck). The test layer is MEASURED, not assumed: tsc -p tsconfig.test.json --listFiles puts all four changed files in a 2270-file program, and the debt ledger held at 54 file(s) / 261 error(s) / 145 pinned signature(s) unchanged.
  • Gate families derived mechanically with scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack rather than from the dispatch list: 73 families. 71 ran green.
  • pnpm lint (eslint . --no-inline-config, the whole repo — no narrowing) — exit 0 at 395a57c3d.
  • Control-character self-scan over all five changed files: 0 hits, with a positive control lit on a planted BEL byte. pnpm check:nul-bytes exit 0.

NOT MEASURED locally — 2 of 73, handed to CI, reconciled with dispatch-gates.mjs --ran which named exactly these two:

  • pnpm check:dual-build-cjs-loads — exit 3, PREREQUISITE NOT MET, needs all 79 packages built.
  • pnpm check:type-check-debt — exit 3, PREREQUISITE NOT MET, needs the full workspace closure built.

Both print "This is NOT a pass and NOT a finding: nothing was measured." A whole-workspace build is the CI farm's run, not a foreground one. Reported as unmeasured, not as green.

Six gates first returned exit 1 or 3 purely because packages/spec was unbuilt (check:api-surface, check:dual-source-exports, check:browser-reachable-entries, check:entry-nameability, check:exported-any, check:doc-formula-expressions). Those are PREREQUISITE NOT MET, not findings; after turbo run build --filter=@objectstack/spec --filter=@objectstack/formula --filter=@objectstack/lint all six re-ran green, and those greens are what the 71 counts.

Scope

The stored-value scan (record-validator.ts, #15490) is deliberately untouched — a different surface with a different audience, as the card and the triage seat both drew it. No adjacent defects were found to file.

⛔ Draft on purpose. Not for ready, not for the queue — the PM's call after review.


🤖 Generated with Claude Code

https://claude.ai/code/session_01T6HeZvT9wdSJD1ZxJb5Eno


Generated by Claude Code

…s issue over a positional read

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6HeZvT9wdSJD1ZxJb5Eno
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 4 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/protocol/objectql/types.mdx (via unrecognized_keys (literal, a string literal in actionableValueIssueMessage))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx (via unrecognized_keys (literal, a string literal in actionableValueIssueMessage))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 130 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 4500bc5d7917324330f062424efebdadc62c6fb7packageMentionDocs.

Which tree this was computed on

This run read content/docs from 1f056ce7fc36fd959ded83d9f5e9b05c5048dc9c — the merge of head edf64d5e596e8f09145ce2325d78a30e23d0cc68 into base 4500bc5d7917324330f062424efebdadc62c6fb7, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 1f056ce7fc36fd959ded83d9f5e9b05c5048dc9c && git checkout 1f056ce7fc36fd959ded83d9f5e9b05c5048dc9c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 4500bc5d7917324330f062424efebdadc62c6fb7 edf64d5e596e8f09145ce2325d78a30e23d0cc68 && git checkout -B drift-repro 4500bc5d7917324330f062424efebdadc62c6fb7 && git merge --no-ff edf64d5e596e8f09145ce2325d78a30e23d0cc68

node scripts/docs-audit/affected-docs.mjs --json 4500bc5d7917324330f062424efebdadc62c6fb7

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 4500bc5d7917324330f062424efebdadc62c6fb7 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@huangyiirene
huangyiirene marked this pull request as ready for review September 6, 2026 19:50
…e-preference

Brings the branch current with main (dff0bdd) and re-fires CI. main's changes
since the merge base touch no file under packages/spec, so this merge carries no
content decision for this PR.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6HeZvT9wdSJD1ZxJb5Eno
…eral-rename-preference

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6HeZvT9wdSJD1ZxJb5Eno

Copy link
Copy Markdown
Collaborator Author

os-dev-report

Branch refresh only — no content change. origin/main was resolved once to the literal sha c14ebfeb29d8d3bc0ae9b148132a78b86dec94bc and that sha was merged by hand (the branch's previous refresh base was dff0bdd3ac, 17 commits behind). Purpose: bring the branch current and re-fire CI after the Test Core (5/6) 30-minute timeout-minutes flake tracked by #16173, which is not this PR's content.

  • merge commit 414cc30592804c922a1911a2a4b3689a0eebdd20, parents 5aba346c4d91fce8d0319c85c5eb4bd76f121462 (branch) and c14ebfeb29d8d3bc0ae9b148132a78b86dec94bc (main). Only new commit on the branch; no rebase, amend or force-push.
  • conflicts: none, and zero files changed on both sides (the merge log carries no Auto-merging line). No merge=os-regen path was touched by this branch, so the regen driver never ran and no deferral marker was recorded.
  • delta vs main unchanged: same 5 files, same +167 / -4, and blob-identical for all five before and after the merge.
  • verdicts, all exit 0: spec dependency-closure + pnpm --filter @objectstack/spec build; pnpm --filter @objectstack/spec test — 482 files / 13109 tests passed; the three pin files (default-value-shape, field-default-value, action-param-default-value) run explicitly — 3 files / 99 tests passed, with all seven #16077-labelled cases reported passing by name under a verbose reporter; pnpm --filter @objectstack/spec typecheck; pnpm --filter @objectstack/spec check:generated — "All 15 generated artifacts are up to date".

No PR body, label, draft-state or auto-merge change was made, and the patch changeset level was left as reasoned.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

the defaultValue literal gate prescribes the missing-member type error, not the key rename the schema carries

2 participants